Remove dependency on deprecated puppetlabs-pe_gem #9
Conversation
Support for puppet 3 was dropped in an earlier commit. From puppetlabs-pe_gem README > As of Puppet 4.0, this module has been deprecated. Please use Puppet > 4.0's built-in puppet_gem provider instead. Since install.pp was a private class with only a single resouce I've moved the declaration of the 'faraday' package resource to init.pp
| gem 'rspec-puppet', :require => false | ||
| gem 'puppetlabs_spec_helper', :require => false | ||
| gem 'puppet-lint', :require => false | ||
| gem 'metadata-json-lint', :require => false |
There was a problem hiding this comment.
Why did you add the gem? If this is a needed dependency for one of the existing gems, then this should be addressed in a separate commit.
There was a problem hiding this comment.
It’s a common mistake to remove the ‘dependencies’ key from the metadata if there are no dependencies. But this would make the metadata invalid, (if there are no dependencies, you still need an empty array). This gem guards against this from happening in the future.
| @@ -1,14 +0,0 @@ | |||
| # Private class | |||
| class f5::install { | |||
| if $::puppetversion and $::puppetversion =~ /Puppet Enterprise/ { | |||
There was a problem hiding this comment.
The better way to handle this would be to leave the f5::install class as many pre-existing users are using this and just comment out the pe_gem dependency.
cc @ericzji
There was a problem hiding this comment.
Really? The f5:install class isn't mentioned in the README and is marked Private class on line 1. I could move the faraday package resource back to this class, but there really did seem little point in keeping it. I also thought committing commented out code was considered bad practice these days?
There was a problem hiding this comment.
Yes, the cisco_ios module uses this model:
https://github.com/puppetlabs/cisco_ios/blob/master/manifests/init.pp
And the device_manager module automatically leverages this model:
https://github.com/puppetlabs/device_manager/blob/master/manifests/init.pp#L76
|
At this point, implementing ... in install.pp is reasonable. |
4 participants
Support for puppet 3 was dropped in an earlier commit.
From puppetlabs-pe_gem README
Since install.pp was a private class with only a single resouce I've
moved the declaration of the 'faraday' package resource to init.pp